Skip to content

Roll back failed and undone inputs, and add a checkpoint for Undo - #1174

Open
conrade-ctc wants to merge 1 commit into
compiler-research:mainfrom
chicagotrading:undo-failed-inputs-upstream
Open

conrade-ctc wants to merge 1 commit into
compiler-research:mainfrom
chicagotrading:undo-failed-inputs-upstream

Conversation

@conrade-ctc

@conrade-ctc conrade-ctc commented Nov 5, 2026 •

Copy link
Copy Markdown
Collaborator

Description

In a notebook, one failed input can break every later input, and the user
must restart. clang-repl removes the declarations of a failed or undone
input, but not all the state that the input changed. The upstream work is in
progress (llvm/llvm-project#223305 and others; the new doc page lists them).
Until it lands, this PR does what CppInterOp can do with the clang API:

  • A failed Declare, Process or Evaluate withdraws its inputs, also when
    it fails to link or to run.
  • Before Execute, a module with static initializers is checked for missing
    symbols. LLJIT keeps the init function of a module that fails to link, and
    every later input then fails.
  • A failed parse resets the implicit instantiations that it requested, as
    [clang-repl] Roll back implicit instantiations on failed PTUs to prevent JIT starvation llvm/llvm-project#192588 does in clang-repl.
  • A failed parse or an undo restores macros and include state.
  • New GetInputCount gives a checkpoint for Undo. Undo also cleans the
    TU scope, reopened namespaces, replaced redeclarations and the enumerators
    of unscoped enums. It drops only the cached wrappers that live in a
    withdrawn input.

Since llvm/llvm-project#218149 (LLVM 23.1.3), clang-repl removes the members
of reopened namespaces itself, and this PR then skips that step. clang-repl
removes an undone enum, but not its enumerators.

docs/UndoAndFailedInputs.rst lists what is not rolled back, and why. I
think each workaround can go when the matching upstream change is in all
supported LLVM versions.

The instantiation, macro and include rollback needs LLVM 22 or later.

One test changes: VariableReflection_GetVariableOffset_UnmaterializableDefinition
relied on a failed Declare that kept its class. The class is now a
template, so the failure stays in the input that GetVariableOffset makes.

Type of change

  • Bug fix
  • New feature
  • Requires documentation updates

Testing

New tests in unittests/CppInterOp/InterpreterTest.cpp:
Interpreter_FailedLink*, Interpreter_FailedParse* and Interpreter_Undo*.
I ran check-cppinterop against LLVM 22 and 23 with assertions, also with
release/23.x, which has llvm/llvm-project#218149.
compiler-research/cppjit#177 adds end-to-end notebook cases.

Checklist

  • I have read the contribution guide recently

🤖 Done with the help of Claude Code (Opus 5.5, human in the loop)

@codecov

codecov Bot commented Nov 5, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.32%. Comparing base (3a6e86f) to head (646f212).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1174      +/-   ##
==========================================
+ Coverage   88.86%   89.32%   +0.46%     
==========================================
  Files          24       25       +1     
  Lines        7014     7318     +304     
==========================================
+ Hits         6233     6537     +304     
  Misses        781      781              
Files with missing lines Coverage Δ
lib/CppInterOp/CppInterOp.cpp 91.58% <100.00%> (+0.36%) ⬆️
lib/CppInterOp/CppInterOpInterpreter.h 91.59% <100.00%> (+1.41%) ⬆️
lib/CppInterOp/InputRollback.cpp 100.00% <100.00%> (ø)
lib/CppInterOp/InterpreterInfo.h 100.00% <ø> (ø)
Files with missing lines Coverage Δ
lib/CppInterOp/CppInterOp.cpp 91.58% <100.00%> (+0.36%) ⬆️
lib/CppInterOp/CppInterOpInterpreter.h 91.59% <100.00%> (+1.41%) ⬆️
lib/CppInterOp/InputRollback.cpp 100.00% <100.00%> (ø)
lib/CppInterOp/InterpreterInfo.h 100.00% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

There were too many comments to post at once. Showing the first 10 out of 28. Check the log or trigger a new build to see more.

Comment thread lib/CppInterOp/CppInterOp.cpp Resolved
Comment thread lib/CppInterOp/CppInterOp.cpp Resolved
Comment thread lib/CppInterOp/CppInterOp.cpp Resolved
Comment thread lib/CppInterOp/CppInterOp.cpp Resolved
Comment thread lib/CppInterOp/CppInterOp.cpp Resolved
Comment thread lib/CppInterOp/CppInterOp.cpp Resolved
Comment thread lib/CppInterOp/CppInterOp.cpp Resolved
Comment thread lib/CppInterOp/CppInterOp.cpp Resolved
Comment thread lib/CppInterOp/CppInterOp.cpp Resolved
Comment thread lib/CppInterOp/CppInterOp.cpp Resolved

@mcbarton mcbarton left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plus you want to add _ZN5clang11Interpreter18getExecutionEngineEv to the list of exported symbols here https://github.com/chicagotrading/CppInterOp/blob/undo-failed-inputs-upstream/lib/CppInterOp/exports.ld . Unless I missed one of the symbols, this should fix your undefined symbol errors for the wasm ci.

Comment thread lib/CppInterOp/CppInterOp.cpp Resolved
Comment thread lib/CppInterOp/InputRollback.cpp Resolved
Comment thread lib/CppInterOp/InputRollback.cpp Resolved
@conrade-ctc
conrade-ctc force-pushed the undo-failed-inputs-upstream branch 2 times, most recently from 14e6dfe to 683a760 Compare November 6, 2026 01:19
@conrade-ctc

Copy link
Copy Markdown
Collaborator Author

Thanks. The link check is the only new code that calls getExecutionEngine, and it is now off for Emscripten. I think the check does not apply there, because the wasm executor does not use LLJIT. So I did not add the symbol to exports.ld.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

Comment thread lib/CppInterOp/InputRollback.h Resolved
Comment thread lib/CppInterOp/InterpreterInfo.h Resolved
@conrade-ctc
conrade-ctc force-pushed the undo-failed-inputs-upstream branch 2 times, most recently from c76eac2 to ddcde50 Compare November 6, 2026 17:03
A failed Declare, Process or Evaluate leaves the interpreter as it was
before the input:

- An input that parses but fails to link or to run is withdrawn.
- Before Execute, a module with static initializers is checked for
  missing symbols. LLJIT keeps the init function of a module that fails
  to link, and every later input then fails.
- The implicit instantiations that a failed input requested are reset.
- Macros and include state are restored.

GetInputCount gives a checkpoint for Undo. Undo also cleans the TU
scope, reopened namespaces, replaced redeclarations and the wrapper
caches.

docs/UndoAndFailedInputs.rst lists what is not rolled back, and the
upstream clang-repl work that can replace these workarounds.

Co-developed-with-the-help-of: Claude Code (Opus 5.5, human in the loop)
@conrade-ctc
conrade-ctc force-pushed the undo-failed-inputs-upstream branch from ddcde50 to 646f212 Compare November 6, 2026 17:14
@mcbarton

mcbarton commented Nov 6, 2026 •

Copy link
Copy Markdown
Collaborator

Ignore any failures of the Windows wasm job. The out of memory issue is known, and #1172 will disable the Chromium tests on Windows for now.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants